Skip to content

types(ModalSubmitInteraction): make attachments optional#11363

Open
rahuld109 wants to merge 2 commits intodiscordjs:mainfrom
rahuld109:fix/modal-attachments-optional
Open

types(ModalSubmitInteraction): make attachments optional#11363
rahuld109 wants to merge 2 commits intodiscordjs:mainfrom
rahuld109:fix/modal-attachments-optional

Conversation

@rahuld109
Copy link

@rahuld109 rahuld109 commented Dec 14, 2025

Summary

Makes FileUploadModalData.attachments optional to match runtime behavior.

Problem

When a modal has an optional FileUpload component and the user doesn't upload a file, component.attachments is undefined at runtime. However, the TypeScript types marked it as required, causing no compile-time warning but runtime errors when accessing the property.

Solution

Add ? to the attachments field in FileUploadModalData interface:

// Before
attachments: ReadonlyCollection<Snowflake, Attachment>;

// After  
attachments?: ReadonlyCollection<Snowflake, Attachment>;

Closes #11359

@rahuld109
types(ModalSubmitInteraction): make attachments optional
fe5f536 
The attachments field can be undefined when a user doesn't upload
a file in an optional FileUpload component, but the types didn't
reflect this possibility, causing runtime errors.

close discordjs#11359
@rahuld109 rahuld109 requested a review from a team as a code owner December 14, 2025 04:23
@vercel vercel bot temporarily deployed to Preview – discord-js-guide December 14, 2025 04:23 Inactive
@vercel
Copy link

vercel bot commented Dec 14, 2025 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for GitHub.

2 Skipped Deployments
Project Deployment Review Updated (UTC)
discord-js Skipped Skipped Dec 22, 2025 9:42am
discord-js-guide Skipped Skipped Dec 22, 2025 9:42am

@vercel vercel bot temporarily deployed to Preview – discord-js December 14, 2025 04:23 Inactive
@coderabbitai
Copy link

coderabbitai bot commented Dec 14, 2025

📝 Walkthrough

Walkthrough

The attachments field in FileUploadModalData is made optional in the TypeScript type definitions to align with runtime behavior where the field may be undefined.

Changes

Cohort / File(s) Summary
Type definition update
packages/discord.js/typings/index.d.ts		 Made attachments field optional in FileUploadModalData interface

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

  • Single file change affecting one field declaration
  • Straightforward type adjustment (required → optional) to reflect actual runtime behavior
  • Minimal risk; verify that no code strictly depends on attachments being non-optional in consumers of FileUploadModalData

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: making attachments optional in the FileUploadModalData type for ModalSubmitInteraction.
Description check ✅ Passed The description is directly related to the changeset, clearly explaining the problem, solution, and the specific type change being made.
Linked Issues check ✅ Passed The PR successfully addresses issue #11359 by making the attachments field optional, matching the runtime behavior where attachments can be undefined.
Out of Scope Changes check ✅ Passed All changes are in scope: only the attachments field in FileUploadModalData is modified to match the objective of issue #11359.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c4fc79a and fe5f536.

📒 Files selected for processing (1)
  • packages/discord.js/typings/index.d.ts (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/discord.js/typings/index.d.ts (1)
packages/collection/src/collection.ts (1)
  • ReadonlyCollection (6-10)
🔇 Additional comments (1)
packages/discord.js/typings/index.d.ts (1)

2564-2568: Correct typing fix; note TS-breaking change for consumers
Making FileUploadModalData.attachments optional matches the observed runtime (undefined when no file is uploaded) and forces proper guards/optional chaining. Consider calling this out in release notes/changelog since it can break downstream TypeScript builds that assumed attachments was always present.

Comment @coderabbitai help to get the list of available commands and usage tips.

@vakiliner vakiliner mentioned this pull request Dec 14, 2025
Closed
3 tasks
almeidx
almeidx previously approved these changes Dec 17, 2025
View reviewed changes
@almeidx almeidx mentioned this pull request Dec 17, 2025
Closed
@vercel vercel bot temporarily deployed to Preview – discord-js December 22, 2025 09:42 Inactive
@vercel vercel bot temporarily deployed to Preview – discord-js-guide December 22, 2025 09:42 Inactive
@vladfrangu
Copy link
Member

vladfrangu commented Jan 2, 2026

Is there a reason we don't just always create a collection? Thoughts?

@Jiralite
Copy link
Member

Jiralite commented Jan 2, 2026

If we receive an empty array, we should make an empty collection.

@rahuld109
Copy link
Author

rahuld109 commented Jan 17, 2026

Just checking - would you like me to switch to the alternate approach (always creating an empty collection at runtime)? Let me know and I can make that change.

@Jiralite
Copy link
Member

Jiralite commented Jan 17, 2026 •
edited
Loading

When a modal has an optional FileUpload component and the user doesn't upload a file, component.attachments is undefined at runtime. However, the TypeScript types marked it as required, causing no compile-time warning but runtime errors when accessing the property.

If Discord sends an empty array, we should be setting components.attachments to an empty collection. Can you verify that behaviour?

Jiralite
Jiralite requested changes Feb 22, 2026
View reviewed changes
Copy link
Member

@Jiralite Jiralite left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have just verified this behaviour:

{
  "type": 18,
  "id": 9,
  "component": {
    "values": [],
    "type": 19,
    "id": 10,
    "custom_id": "164"
  }
}

An empty array is sent for values. The types are fine, it's JavaScript that needs to be fixed–an empty collection should be used.

@github-project-automation github-project-automation bot moved this from Todo to Review in Progress in discord.js Feb 22, 2026
@Jiralite Jiralite removed the typings label Feb 22, 2026
@Jiralite Jiralite added this to the discord.js 15.0.0 milestone Feb 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Jiralite Jiralite Jiralite requested changes

@almeidx almeidx almeidx left review comments

@vladfrangu vladfrangu Awaiting requested review from vladfrangu vladfrangu is a code owner automatically assigned from discordjs/core

@iCrawl iCrawl Awaiting requested review from iCrawl iCrawl is a code owner automatically assigned from discordjs/core

@kyranet kyranet Awaiting requested review from kyranet kyranet is a code owner automatically assigned from discordjs/core

@SpaceEEC SpaceEEC Awaiting requested review from SpaceEEC SpaceEEC is a code owner automatically assigned from discordjs/core

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

backport-candidate in review packages:discord.js semver:patch

Projects

discord.js
Status: Review in Progress

Milestone

discord.js 15.0.0

Development

Successfully merging this pull request may close these issues.

ModalSubmitInteraction field component.attachments may be undefined despite typings

4 participants

@rahuld109 @vladfrangu @Jiralite @almeidx